Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libvpx] Bump to 1.13.1 #35047

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Conversation

basilgello
Copy link
Contributor

@basilgello basilgello commented Nov 11, 2023

Based on #34814 by @LilyWangLL
Fixes #34809
but tailored for RustDesk to build on arm64-ios, arm*-linux etc.

Signed-off-by: Vasyl Gello [email protected]

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@basilgello
Copy link
Contributor Author

Wondering how many builds fail with current revision of libvpx MR but dont want hijacking @LilyWangLL :)

@basilgello basilgello force-pushed the libvpx-1.13.1 branch 7 times, most recently from 15c1604 to b030b12 Compare November 11, 2023 21:46
@basilgello basilgello marked this pull request as ready for review November 12, 2023 10:42
@LilyWangLL LilyWangLL mentioned this pull request Nov 13, 2023
7 tasks
@JonLiu1993 JonLiu1993 added the category:port-update The issue is with a library, which is requesting update new revision label Nov 13, 2023
JonLiu1993
JonLiu1993 previously approved these changes Nov 13, 2023
@JonLiu1993
Copy link
Member

All features are tested successfully in the following triplet:

  • x86-windows
  • x64-windows
  • x64-windows-static

Tested usage successfully by libvpx[*]:x64-windows:

libvpx provides CMake targets:

  # this is heuristically generated, and may not be correct
  find_package(unofficial-libvpx CONFIG REQUIRED)
  target_link_libraries(main PRIVATE unofficial::libvpx::libvpx)

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Nov 13, 2023
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unofficial-libvpx.config.cmake.in has whitespace-only changes. Please restore the last revision of this file.

WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-rel"
LOGNAME configure-${TARGET_TRIPLET}-rel)

message(STATUS "Building libvpx for Release")
vcpkg_execute_required_process(
COMMAND
${BASH} --noprofile --norc -c "make -j8"
${BASH} --noprofile --norc -c "make -j"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
${BASH} --noprofile --norc -c "make -j"
${BASH} --noprofile --norc -c "make -j${VCPKG_CONCURRENCY}"

WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg"
LOGNAME configure-${TARGET_TRIPLET}-dbg)

message(STATUS "Building libvpx for Debug")
vcpkg_execute_required_process(
COMMAND
${BASH} --noprofile --norc -c "make -j8"
${BASH} --noprofile --norc -c "make -j"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
${BASH} --noprofile --norc -c "make -j"
${BASH} --noprofile --norc -c "make -j${VCPKG_CONCURRENCY}"

@basilgello
Copy link
Contributor Author

@dg0yt Addressed the points! Thanks!

@JonLiu1993 JonLiu1993 removed the info:reviewed Pull Request changes follow basic guidelines label Nov 13, 2023
@basilgello
Copy link
Contributor Author

basilgello commented Nov 13, 2023

And fixed the remnants from old crossbuild fix. Tested on arm64-linux and arm-linux, as well as arm64-android amd arm-neon-android

@dg0yt
Copy link
Contributor

dg0yt commented Nov 13, 2023

... if we only could use vcpkg_configure_make and friends...

@basilgello
Copy link
Contributor Author

basilgello commented Nov 13, 2023

vpx is special :( Properly this needs a cmake file just like we do for selected dependencies in Kodi. Ideally, merged into upstream. But this is separate effort and we are missing security update right now.

@dg0yt
Copy link
Contributor

dg0yt commented Nov 13, 2023

For other "special" ports such as openssl, I added configure wrapper scripts.

JonLiu1993
JonLiu1993 previously approved these changes Nov 13, 2023
@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Nov 13, 2023
@basilgello
Copy link
Contributor Author

@dg0yt If I understand the openssl port correctly, you created the wrapper so that canonical configure invocation is transformed to openssl's nonstandard flags?

@dg0yt
Copy link
Contributor

dg0yt commented Nov 14, 2023

@dg0yt If I understand the openssl port correctly, you created the wrapper so that canonical configure invocation is transformed to openssl's nonstandard flags?

Yes. Plus the openssl particularity of Configure being a Perl script, spelled like that...
The motivation is to leverage all the toolchain detection and forwarding implemented in vcpkg_configure/build/install_make, instead of maintaining another poor copy.

Other examples are botan, libcap, luajit. Some packages don't have configure, but ask for variables to be passed directly to make. Then the port's configure script captures configuration input (e.g. --prefix, --enable-shared) and generates a top-level Makefile hard-coding the parameters of interest.

You don't have to add this to this PR... I just want to indicate that there is an alternative.

@basilgello
Copy link
Contributor Author

I am not going to spoil this PR as this is a security update. Port version 1 is another story :)

data-queue
data-queue previously approved these changes Nov 15, 2023
set(LIBVPX_TARGET "generic-gnu")
# Settings
if(VCPKG_TARGET_ARCHITECTURE STREQUAL x86)
set(ANDROID_TARGET_TRIPLET i686-linux-android)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, another leftover from my attempt to use "proper" new-style invocation of NDK.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android NDK... that's another blocker for vcpkg_configure_make, since r26. #31228 😢

{
"name": "vcpkg-msbuild",
"host": true,
"platform": "windows"
},
{
"name": "yasm",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reasoning for adding this dependency, as opposed to using NASM?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By defailt, yasm is preferred by upstream: https://github.com/webmproject/libvpx/blob/main/configure#L33

Plus I recall I had issues with nasm on non-windows architectures but I can cross-check once more

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far yasm can be dropped safely :)

@data-queue data-queue marked this pull request as draft November 15, 2023 00:33
@JonLiu1993 JonLiu1993 removed the info:reviewed Pull Request changes follow basic guidelines label Nov 15, 2023
Based on microsoft#34814 by @LilyWangLL
but tailored for RustDesk to build on arm64-ios, arm*-linux etc.

Signed-off-by: Vasyl Gello <[email protected]>
@basilgello basilgello dismissed stale reviews from data-queue and JonLiu1993 via 83a712a November 15, 2023 10:07
@basilgello basilgello marked this pull request as ready for review November 15, 2023 12:49
@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Nov 16, 2023
@data-queue data-queue merged commit ac2a14f into microsoft:master Nov 16, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libvpx] update to 1.13.1
4 participants